Skip to content

chore: refactor header shortcuts#2372

Merged
43081j merged 6 commits intomainfrom
jg/keyboard-longcuts
Apr 4, 2026
Merged

chore: refactor header shortcuts#2372
43081j merged 6 commits intomainfrom
jg/keyboard-longcuts

Conversation

@43081j
Copy link
Copy Markdown
Contributor

@43081j 43081j commented Apr 3, 2026

🔗 Linked issue

N/A

📚 Description

Just a simplification since these are getting lengthy and repetitive
already.

@43081j
chore: refactor header shortcuts
9d52025 
Just a simplification since these are getting lengthy and repetitive
already.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 3, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Apr 4, 2026 7:07pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Apr 4, 2026 7:07pm
npmx-lunaria Ignored Ignored Apr 4, 2026 7:07pm

Request Review

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 50.00000% with 18 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/composables/useShortcuts.ts 50.00% 10 Missing and 4 partials ⚠️
app/components/AppHeader.vue 0.00% 2 Missing ⚠️
app/components/Package/Header.vue 66.66% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f7aba753-dc9f-4c3a-8734-c77e1b826dcd

📥 Commits

Reviewing files that changed from the base of the PR and between 5c21dc8 and b369d3c.

📒 Files selected for processing (1)
  • app/composables/useShortcuts.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/composables/useShortcuts.ts
📝 Walkthrough

Walkthrough

Centralises keyboard shortcut handling by adding a new useShortcuts composable and initKeyShortcuts() initialiser. Per-component onKeyStroke handlers were removed and replaced by declarative useShortcuts({...}) registrations in components (Package Header, AppHeader, app.vue) that return route targets; global handling resolves the most-recently-registered target, prevents default, and navigates via navigateTo. Editable-element filtering and direct router.push usage were removed; registrations are cleaned up on scope dispose. initKeyShortcuts() is invoked during app startup.

Possibly related PRs

  • npmx-dev/npmx.dev PR 1039: Modifies app-wide and component keyboard shortcut handling, replacing prior manual keydown handlers with centralized utilities.
  • npmx-dev/npmx.dev PR 2030: Changes the Package/Header component and its keyboard-accessible controls, overlapping the header shortcut refactor in this change.
  • npmx-dev/npmx.dev PR 1684: Centralises keyboard-shortcut registration and integrates gating logic, closely related to the new initKeyShortcuts/useShortcuts flow.

Suggested reviewers

  • danielroe
  • ghostdevv
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description directly relates to the changeset, explaining the refactoring of header shortcuts for simplification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg/keyboard-longcuts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/components/Package/Header.vue (1)

124-143: Clean refactor consolidating keyboard shortcut handlers.

The loop-driven approach reduces duplication and improves maintainability. The uniform "compute → check → preventDefault → navigate" pattern is clear.

One minor nitpick: the type annotation includes false, but the expression on line 128 (props.pkg && {...}) can only return null, undefined, or the object—never false. You could simplify the type:

🔧 Tighten the type annotation 
-const shortcuts: [string, () => RouteLocationRaw | null | false | undefined][] = [
+const shortcuts: [string, () => RouteLocationRaw | null | undefined][] = [
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 45267a1e-308c-41d4-9ddf-d7b804dc0930

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1f450 and 9d52025.

📒 Files selected for processing (1)
  • app/components/Package/Header.vue

coderabbitai[bot]
coderabbitai bot reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/components/AppHeader.vue (1)

201-204: Keep the header shortcut mapping single-sourced.

The c and , routes are now declared here and in desktopLinks, so a future key or target change can desynchronise the visible aria-keyshortcuts metadata from the handler logic. Hoisting these two definitions into one shared constant would keep this refactor aligned with its “less repetition” goal.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b5c8189f-6b58-4efa-a928-c5242f2bbeca

📥 Commits

Reviewing files that changed from the base of the PR and between 9d52025 and 3b214f8.

📒 Files selected for processing (4)
  • app/app.vue
  • app/components/AppHeader.vue
  • app/components/Package/Header.vue
  • app/composables/useShortcuts.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Package/Header.vue

@ghostdevv ghostdevv marked this pull request as draft April 4, 2026 06:52
@ghostdevv ghostdevv marked this pull request as ready for review April 4, 2026 18:31
@ghostdevv
Copy link
Copy Markdown
Contributor

ghostdevv commented Apr 4, 2026

merge when you're ready @43081j

@43081j 43081j enabled auto-merge April 4, 2026 19:05
@43081j 43081j added this pull request to the merge queue Apr 4, 2026
Merged via the queue into main with commit b2ea9bc Apr 4, 2026
23 checks passed
@43081j 43081j deleted the jg/keyboard-longcuts branch April 4, 2026 19:13
@github-actions github-actions bot mentioned this pull request Apr 4, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@ghostdevv ghostdevv ghostdevv approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@43081j @ghostdevv